Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SuperTextField] Add support for inline widgets (Resolves #2507) #2520

Merged
merged 6 commits into from
Jan 25, 2025

Conversation

angelosilvestre
Copy link
Collaborator

[SuperTextField] Add support for inline widgets. Resolves #2507

We already implemented support for inline widgets in SuperEditor. This PR adds support for inline widgets in SuperTextField.

We have a TextSpan buildTextSpan(AttributionStyleBuilder styleBuilder) method inside AttributedTextEditingController. This method doesn't seem to belong here, and it isn't being used in the repo. I marked this method as deprecated.

SuperTextFieldInspector.findRichText().style!.color,
Colors.orange,
);
for (int i = 0; i <= 9; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there something we can do to make this simpler? Doing this once or twice isn't terrible, but in theory one might want to verify details like text color for any number of cases.

In this particular case, it looks like there's a span per index? That doesn't seem correct. If we have a series of characters with identical styles, they should still be available within a single span, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this particular case, it looks like there's a span per index? That doesn't seem correct. If we have a series of characters with identical styles, they should still be available within a single span, right?

We do have a single span, the goal of this change is to make sure that every character has the same color. If the span at offset zero has the right color, but, somehow, we have other spans with other colors, this test would pass.

We could modify this to ensure that the InlineSpan has only one child span.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we usually check for exact matches of span markers, which then ensures that only two color markers exist: start and end. And also ensures no other markers exist at all?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But span markers are at the attributed text level. This is at the Flutter InlineSpan level.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. We can leave it as-is.

final inlineWidgetRect = tester.getRect(find.byPlaceholderName('1'));
expect(
inlineWidgetRect.left,
_getOffsetAtPosition(tester, const TextPosition(offset: 5)).dx,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this offset the left side or the right side of the character rectangle?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the left side.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we should probably switch that to the right side, right? If the widget overlaps the final character then that would be a bug.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, offset 5 IS the inline widget. Maybe we should check if it has a bigger offset than the character at offset 4. I update this.

@@ -0,0 +1,412 @@
import 'package:flutter/gestures.dart';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also have a few golden tests to check caret position, caret height, as well as selection rectangles when selecting over and near inline widgets? These details would be relevant both for single-line text blocks and multi-line text blocks within a text field.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added golden tests.

@angelosilvestre angelosilvestre force-pushed the 2507_supertextfield-inline-widgets branch from 8083e7a to 8360abc Compare January 25, 2025 20:08
@matthew-carroll matthew-carroll merged commit c3106bb into main Jan 25, 2025
16 checks passed
@matthew-carroll matthew-carroll deleted the 2507_supertextfield-inline-widgets branch January 25, 2025 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SuperTextField] - Bring inline widgets to SuperTextField
2 participants